envoy: initial boilerplate/skeleton for metadata upstream#3245
envoy: initial boilerplate/skeleton for metadata upstream#3245istio-testing merged 1 commit intoistio:masterfrom
Conversation
google-cla
bot
added
the
cla: yes
istio-testing
added
the
size/L
|
/retest |
lambdai
left a comment
lambdai
left a comment
There was a problem hiding this comment.
Nice move!
Can you add an integration test to prove that this upstream extension can be injected into the envoy cluster?
PiotrSikora
left a comment
PiotrSikora
left a comment
There was a problem hiding this comment.
Is there any reason why this is added to Istio Proxy and not Envoy itself?
That can't be done yet. It will require more code (connection pool, connection pool factory). I think that should be left for future PRs to keep size small
I think adding this to Envoy would require aligning on an API that would be extensible enough to be used for needs in addition to just Istio. envoyproxy/envoy#15472 is for discussing this. If that discussion blocked starting this, I don't think there would be time to finish this before Istio 1.10. This code would not include the extensible API, but just the headers that are needed for Istio/BTS (original port, maybe telemetry) |
PiotrSikora
left a comment
PiotrSikora
left a comment
There was a problem hiding this comment.
Consensus from the offline discussion is to work on this upstream.
/hold
PiotrSikora
left a comment
PiotrSikora
left a comment
There was a problem hiding this comment.
Apparently, Envoy doesn't want this.
|
/retest |
|
/test test_proxy |
|
/test test-tsan_proxy |
1 similar comment
|
/test test-tsan_proxy |
|
/retest |
4 participants
What this PR does / why we need it:
Initial commit for istio/istio#31444
Based on lambdai/envoy-dai#7
Special notes for your reviewer:
I'd rather keep the commits small to make things easier to review. This is meant to be the smallest testable unit, establishing the code location and some naming